[DataGrid] Add fluid columns width support #480

DanailH · 2020-10-21T15:02:43Z

This PR aims to provide support for fluid / responsive columns. This is achieved by setting a flex property on a column definition. The flex property can work together with the width property.

I've tested a couple of data grids to benchmark the functionality and the one that seemed the most intuitive was the one that ag-Grid provides.

The default functionality is not affected by this change. The fluid columns will only work if a user sets the flex optional property in the column definition.

Limitations: Currently if you resize a column with flex and you resize the entire page the sizes of all columns are returned to their original state (this is also the current behavior)

PS: Although this solution works if we decide to go with a different public API I will change it.

Fixes #347

oliviertassinari

@amiranga Does the new API solve your problem? You can preview the documentation at https://deploy-preview-480--material-ui-x.netlify.app/components/data-grid/columns/#column-fluid-width

@DanailH for the test side, this feature sounds like a (rare?) case where we could leverage unit tests on the width computation logic rather than using a full end-to-end suite. We could probably have one smoke end to end test in karma (we need to make sure the widths in pixels are correct, screenshots would be bad, and we don't need to have interactions), and cover the edge cases with unit tests? I wonder 🤔

docs/src/pages/components/data-grid/columns/columns.md

oliviertassinari · 2020-10-21T16:45:17Z

We would need to change the demo or introduce the concept of minWidth, the first column is unusable:

(or at least have #416 😁)

DanailH · 2020-10-22T08:48:15Z

We would need to change the demo or introduce the concept of minWidth, the first column is unusable:

(or at least have #416 😁)

Yes, I agree with this and we can easily bump the minWidth. Currently its at 50px we can make it a bit bigger than that, what do you think @oliviertassinari ?

…andarts

DanailH · 2020-10-22T09:19:05Z

@amiranga Does the new API solve your problem? You can preview the documentation at https://deploy-preview-480--material-ui-x.netlify.app/components/data-grid/columns/#column-fluid-width

@DanailH for the test side, this feature sounds like a (rare?) case where we could leverage unit tests on the width computation logic rather than using a full end-to-end suite. We could probably have one smoke end to end test in karma (we need to make sure the widths in pixels are correct, screenshots would be bad, and we don't need to have interactions), and cover the edge cases with unit tests? I wonder 🤔

Sounds like a plan. The unit tests will check if the width of the columns is calculated correctly after rendering the grid. After all the column calculation when the flex is set is a linear formula with 1 unknown.

I'm more concerned about what should we do about the minWidth. How it currently works is that I'm taking the sum of the widths of all columns that DOES NOT have flex (which by default is 100 px) set and if the sum is more or equal to the total grid width the width of the columns that have flex defaults to the base value of 100 px and a scroll may appear (depends on the total size). If on the other hand, the sum of the width of the columns is less than the width of the grid then the available space is distributed among the columns that do have flex set. That's why in the end some columns with flex may end up with a width of 10px for example. But I can also argue that this is something that can be handled externally.

If we would want to introduce a minWidth property on a column configuration I would suggest creating a separate ticket for it because we would also need to handle the case with the useColumnResize, what do you think @oliviertassinari @dtassone ?

oliviertassinari · 2020-10-22T10:02:01Z

@DanailH Interesting, better ask insights from @dtassone.

oliviertassinari

Unrelated, I have noticed a bug with the hover logic (persistence of hover state on mobile) can be fixed with:

diff --git a/packages/grid/_modules_/grid/components/styled-wrappers/GridRootStyles.ts b/packages/grid/_modules_/grid/components/styled-wrappers/GridRootStyles.ts
index fbe307c2..4e0f7bae 100644
--- a/packages/grid/_modules_/grid/components/styled-wrappers/GridRootStyles.ts
+++ b/packages/grid/_modules_/grid/components/styled-wrappers/GridRootStyles.ts
@@ -126,6 +126,10 @@ export const useStyles = makeStyles(
           cursor: 'col-resize',
           '&:hover, &.Mui-resizing': {
             color: theme.palette.text.primary,
+            // Reset on touch devices, it doesn't add specificity
+            '@media (hover: none)': {
+              color: borderColor,
+            },
           },
         },
         '& .MuiDataGrid-iconSeparator': {

packages/grid/_modules_/grid/hooks/root/useColumns.ts

packages/grid/_modules_/grid/models/colDef/colDef.ts

packages/grid/_modules_/grid/hooks/root/useColumns.ts

Co-authored-by: Olivier Tassinari <olivier.tassinari@gmail.com>

…:DanailH/material-ui-x into feature/DataGrid-347-columns-fluid-width

…toHaveInlineStyle

DanailH · 2020-10-28T17:14:21Z

Unrelated, I have noticed a bug with the hover logic (persistence of hover state on mobile) can be fixed with:

diff --git a/packages/grid/_modules_/grid/components/styled-wrappers/GridRootStyles.ts b/packages/grid/_modules_/grid/components/styled-wrappers/GridRootStyles.ts
index fbe307c2..4e0f7bae 100644
--- a/packages/grid/_modules_/grid/components/styled-wrappers/GridRootStyles.ts
+++ b/packages/grid/_modules_/grid/components/styled-wrappers/GridRootStyles.ts
@@ -126,6 +126,10 @@ export const useStyles = makeStyles(
           cursor: 'col-resize',
           '&:hover, &.Mui-resizing': {
             color: theme.palette.text.primary,
+            // Reset on touch devices, it doesn't add specificity
+            '@media (hover: none)': {
+              color: borderColor,
+            },
           },
         },
         '& .MuiDataGrid-iconSeparator': {

Done

docs/src/pages/components/data-grid/columns/columns.md

dtassone · 2020-10-30T11:56:13Z

Missing storybook stories. Behaviour with many cols, with just a few...
What happens when we resize the grid?

What happens if we resize the cols?

dtassone · 2020-10-30T11:57:51Z

packages/grid/_modules_/grid/hooks/features/columns/useColumns.ts

@@ -16,14 +16,47 @@ import { useApiMethod } from '../../root/useApiMethod';
 import { Logger, useLogger } from '../../utils/useLogger';
 import { useGridState } from '../core/useGridState';

+function mapColumns(


Should this be selector?

I would keep it as it is. This function is only used in the hydrateColumns one and initially, that functionality was part of the hydrateColumns but I separated it from it because the hydrateColumns was becoming a bit too bloated.

docs/src/pages/components/data-grid/columns/columns.md

packages/grid/data-grid/src/DataGrid.test.tsx

Co-authored-by: Matt <github@nospam.33m.co>

…:DanailH/material-ui-x into feature/DataGrid-347-columns-fluid-width

docs/src/pages/components/data-grid/columns/columns.md

updated

DanailH · 2020-11-04T12:47:00Z

Should I merge this or should I wait for everyone to approve? @mbrookes @dtassone

dtassone · 2020-11-04T14:38:42Z

Should I merge this or should I wait for everyone to approve? @mbrookes @dtassone

Thanks for waiting. ATM I would say, wait for the approval of Olivier and me, please :)

I always do that's why I'm asking 😋

oliviertassinari · 2020-11-05T11:34:59Z

packages/grid/_modules_/grid/hooks/features/columns/useColumns.ts

@@ -115,7 +149,7 @@ const getUpdatedColumnState = (
 export function useColumns(columns: Columns, apiRef: ApiRef): InternalColumns {
  const logger = useLogger('useColumns');
  const [gridState, setGridState, forceUpdate] = useGridState(apiRef);
-
+  const viewportWidth = gridState.containerSizes ? gridState.containerSizes.viewportSize.width : 0;


I think that it would be better to keep the same name for variables, const containerWidth would match its usage.

This reverts commit 4cbde43.

@ts-expect-error

* [DataGrid] Add fluid columns width support * Fix doc formatting * Fix documentation so that it is complient to the MUI documentation standarts * Add unit tests * Update packages/grid/_modules_/grid/hooks/root/useColumns.ts Co-authored-by: Olivier Tassinari <olivier.tassinari@gmail.com> * Update packages/grid/_modules_/grid/models/colDef/colDef.ts Co-authored-by: Olivier Tassinari <olivier.tassinari@gmail.com> * Fix PR comments * Fix unit tests * Resolve TS issues * Add // @ts-expect-error need to migrate helpers to TypeScript before toHaveInlineStyle * Add storybook examples * Update docs/src/pages/components/data-grid/columns/columns.md Co-authored-by: Matt <github@nospam.33m.co> * Update docs/src/pages/components/data-grid/columns/columns.md Co-authored-by: Matt <github@nospam.33m.co> * Update docs/src/pages/components/data-grid/columns/columns.md Co-authored-by: Matt <github@nospam.33m.co> * Update docs/src/pages/components/data-grid/columns/columns.md Co-authored-by: Matt <github@nospam.33m.co> * Update docs/src/pages/components/data-grid/columns/columns.md Co-authored-by: Matt <github@nospam.33m.co> * Update packages/grid/data-grid/src/DataGrid.test.tsx Co-authored-by: Matt <github@nospam.33m.co> * Fix storybook flex col width examples * rerun ci * Fix formatting * Update docs/src/pages/components/data-grid/columns/columns.md * Trigger CI Co-authored-by: Olivier Tassinari <olivier.tassinari@gmail.com> Co-authored-by: Matt <github@nospam.33m.co>

This reverts commit 4cbde43.

AleKrabbe · 2021-03-03T16:40:58Z

Sorry to be asking this here, but it seems related. Is there some sort of solution to make the XGrid table responsive by setting column priorities so that they get hidden based on their priority and width of the screen?

Here is an example: http://www.primefaces.org:8080/showcase/ui/data/datatable/responsive.xhtml?jfwid=2d38a

oliviertassinari · 2021-03-03T17:34:52Z

@AleKrabbe Please open a new issue, it's not related to this effort. Currently, you can implement this with the useMediaQuery helper to match specific breakpoints.

[DataGrid] Add fluid columns width support

b2392dd

DanailH requested review from dtassone and oliviertassinari October 21, 2020 15:02

DanailH self-assigned this Oct 21, 2020

oliviertassinari reviewed Oct 21, 2020

View reviewed changes

docs/src/pages/components/data-grid/columns/columns.md Outdated Show resolved Hide resolved

docs/src/pages/components/data-grid/columns/columns.md Outdated Show resolved Hide resolved

docs/src/pages/components/data-grid/columns/columns.md Outdated Show resolved Hide resolved

oliviertassinari added component: data grid This is the name of the generic UI component, not the React module! new feature New feature or request labels Oct 21, 2020

Fix doc formatting

e54cb9a

Fix documentation so that it is complient to the MUI documentation st…

c77a475

…andarts

Add unit tests

756932d

oliviertassinari reviewed Oct 27, 2020

View reviewed changes

packages/grid/_modules_/grid/hooks/root/useColumns.ts Outdated Show resolved Hide resolved

packages/grid/_modules_/grid/models/colDef/colDef.ts Outdated Show resolved Hide resolved

packages/grid/_modules_/grid/hooks/root/useColumns.ts Outdated Show resolved Hide resolved

DanailH and others added 8 commits October 27, 2020 17:32

Update packages/grid/_modules_/grid/hooks/root/useColumns.ts

2d23e7d

Co-authored-by: Olivier Tassinari <olivier.tassinari@gmail.com>

Update packages/grid/_modules_/grid/models/colDef/colDef.ts

6cdced3

Co-authored-by: Olivier Tassinari <olivier.tassinari@gmail.com>

Merge branch 'feature/DataGrid-347-columns-fluid-width' of github.com…

b6cf786

…:DanailH/material-ui-x into feature/DataGrid-347-columns-fluid-width

Fix PR comments

12fa1fa

Rebase from upstream master

3b1d8c3

Fix unit tests

9b042c6

Resolve TS issues

e92df89

Add // @ts-expect-error need to migrate helpers to TypeScript before …

ff5e963

…toHaveInlineStyle

DanailH requested a review from oliviertassinari October 28, 2020 17:14

oliviertassinari approved these changes Oct 28, 2020

View reviewed changes

oliviertassinari requested a review from mbrookes October 28, 2020 17:21

dtassone reviewed Oct 30, 2020

View reviewed changes

docs/src/pages/components/data-grid/columns/columns.md Outdated Show resolved Hide resolved

dtassone reviewed Oct 30, 2020

View reviewed changes

mbrookes previously requested changes Oct 30, 2020

View reviewed changes

DanailH and others added 10 commits November 2, 2020 11:44

Update docs/src/pages/components/data-grid/columns/columns.md

6e5401c

Co-authored-by: Matt <github@nospam.33m.co>

Update docs/src/pages/components/data-grid/columns/columns.md

bf768d3

Co-authored-by: Matt <github@nospam.33m.co>

Update docs/src/pages/components/data-grid/columns/columns.md

94404aa

Co-authored-by: Matt <github@nospam.33m.co>

Update docs/src/pages/components/data-grid/columns/columns.md

4c6a988

Co-authored-by: Matt <github@nospam.33m.co>

Update docs/src/pages/components/data-grid/columns/columns.md

04f336a

Co-authored-by: Matt <github@nospam.33m.co>

Update packages/grid/data-grid/src/DataGrid.test.tsx

b358751

Co-authored-by: Matt <github@nospam.33m.co>

Fix storybook flex col width examples

2219564

rerun ci

401b7ed

Fix formatting

2bd46a0

Merge branch 'feature/DataGrid-347-columns-fluid-width' of github.com…

28baf68

…:DanailH/material-ui-x into feature/DataGrid-347-columns-fluid-width

DanailH requested review from mbrookes and dtassone November 2, 2020 14:40

oliviertassinari approved these changes Nov 2, 2020

View reviewed changes

mbrookes reviewed Nov 2, 2020

View reviewed changes

docs/src/pages/components/data-grid/columns/columns.md Outdated Show resolved Hide resolved

mbrookes and others added 2 commits November 2, 2020 19:58

Update docs/src/pages/components/data-grid/columns/columns.md

9672852

Trigger CI

d300dfe

dtassone approved these changes Nov 4, 2020

View reviewed changes

DanailH merged commit 4cbde43 into mui:master Nov 4, 2020

oliviertassinari reviewed Nov 5, 2020

View reviewed changes

oliviertassinari added a commit that referenced this pull request Nov 6, 2020

Revert "[DataGrid] Add fluid columns width support (#480)"

e8b72d2

This reverts commit 4cbde43.

oliviertassinari mentioned this pull request Nov 6, 2020

[test] Karma should fail if errors are thrown #543

Merged

dtassone pushed a commit to dtassone/material-ui-x that referenced this pull request Nov 9, 2020

Revert "[DataGrid] Add fluid columns width support (mui#480)"

0b1e011

This reverts commit 4cbde43.

oliviertassinari mentioned this pull request Nov 10, 2020

[DataGrid] Add fluid columns width support #566

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[DataGrid] Add fluid columns width support #480

[DataGrid] Add fluid columns width support #480

DanailH commented Oct 21, 2020 •

edited by oliviertassinari

Loading

oliviertassinari left a comment

oliviertassinari commented Oct 21, 2020 •

edited

Loading

DanailH commented Oct 22, 2020 •

edited by oliviertassinari

Loading

DanailH commented Oct 22, 2020 •

edited

Loading

oliviertassinari commented Oct 22, 2020

oliviertassinari left a comment

DanailH commented Oct 28, 2020

dtassone commented Oct 30, 2020 •

edited

Loading

dtassone Oct 30, 2020

DanailH Oct 30, 2020

DanailH commented Nov 4, 2020

dtassone commented Nov 4, 2020 •

edited by DanailH

Loading

oliviertassinari Nov 5, 2020 •

edited

Loading

AleKrabbe commented Mar 3, 2021

oliviertassinari commented Mar 3, 2021

[DataGrid] Add fluid columns width support #480

[DataGrid] Add fluid columns width support #480

Conversation

DanailH commented Oct 21, 2020 • edited by oliviertassinari Loading

oliviertassinari left a comment

Choose a reason for hiding this comment

oliviertassinari commented Oct 21, 2020 • edited Loading

DanailH commented Oct 22, 2020 • edited by oliviertassinari Loading

DanailH commented Oct 22, 2020 • edited Loading

oliviertassinari commented Oct 22, 2020

oliviertassinari left a comment

Choose a reason for hiding this comment

DanailH commented Oct 28, 2020

dtassone commented Oct 30, 2020 • edited Loading

dtassone Oct 30, 2020

Choose a reason for hiding this comment

DanailH Oct 30, 2020

Choose a reason for hiding this comment

DanailH commented Nov 4, 2020

dtassone commented Nov 4, 2020 • edited by DanailH Loading

oliviertassinari Nov 5, 2020 • edited Loading

Choose a reason for hiding this comment

AleKrabbe commented Mar 3, 2021

oliviertassinari commented Mar 3, 2021

DanailH commented Oct 21, 2020 •

edited by oliviertassinari

Loading

oliviertassinari commented Oct 21, 2020 •

edited

Loading

DanailH commented Oct 22, 2020 •

edited by oliviertassinari

Loading

DanailH commented Oct 22, 2020 •

edited

Loading

dtassone commented Oct 30, 2020 •

edited

Loading

dtassone commented Nov 4, 2020 •

edited by DanailH

Loading

oliviertassinari Nov 5, 2020 •

edited

Loading